-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restrict Notifications to secure contexts. #176
Conversation
(this will be even simpler with w3c/permissions@a2464fb) |
Conveniently, this handles the secure context check for us, via <dfn>get the current permissions state</dfn>.
1736a7d
to
78937c4
Compare
PTAL @annevk @marcoscaceres |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Just a couple of nits.
Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
I tried to think through this again. How come |
Hmm, you're right - this isn't quite what we want. I think we should call https://w3c.github.io/permissions/#dfn-request-permission-to-use directly inside |
…> "notifications"
Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
Thanks a lot @miketaylr for getting this cleaned up! |
Thanks for patiently guiding me through it. :) |
This resolves a build issue after whatwg/notifications#176.
This resolves a build issue after whatwg/notifications#176.
Continuing the mess I made in #175.
Fixes #93
Preview | Diff